Skip to content

[CORE][VL] Fix protobuf memory leak in JNI_OnUnload#11532

Merged
PHILO-HE merged 1 commit intoapache:mainfrom
clee704:fix-protobuf-memory-leak
Feb 4, 2026
Merged

[CORE][VL] Fix protobuf memory leak in JNI_OnUnload#11532
PHILO-HE merged 1 commit intoapache:mainfrom
clee704:fix-protobuf-memory-leak

Conversation

@clee704
Copy link
Contributor

@clee704 clee704 commented Jan 31, 2026

What changes were proposed in this pull request?

Add google::protobuf::ShutdownProtobufLibrary() call at the end of JNI_OnUnload() to properly clean up protobuf's static internal data structures when the JNI library is unloaded.

Why are the changes needed?

When running tests with LeakSanitizer enabled, a memory leak is detected:

==4790==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 88 byte(s) in 1 object(s) allocated from:
    #0 operator new(unsigned long) asan_new_delete.cpp
    #1 gluten::ConfigMap::_InternalParse (libgluten.so+0x9ebd49)

Gluten uses protobuf for parsing ConfigMap and Substrait plans. When the protobuf library initializes, it creates static internal data structures. According to the protobuf documentation, dynamically-loaded libraries that use protobuf must call google::protobuf::ShutdownProtobufLibrary() during cleanup to free these static-duration objects when the library is unloaded.

Does this PR introduce any user-facing change?

No. This only adds cleanup code that runs during library unload.

How was this patch tested?

The fix resolves the LeakSanitizer report mentioned above.

Closes #11531

Add google::protobuf::ShutdownProtobufLibrary() call to properly clean up
protobuf's static internal data structures when the JNI library is unloaded.
This fixes memory leak reports from LeakSanitizer.

Closes apache#11531
@github-actions github-actions bot added the VELOX label Jan 31, 2026
@PHILO-HE PHILO-HE changed the title [Core] Fix protobuf memory leak in JNI_OnUnload [CORE][VL] Fix protobuf memory leak in JNI_OnUnload Jan 31, 2026
Copy link
Member

@PHILO-HE PHILO-HE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clee704, thanks for the fix. Maybe, we can enable the sanitizer check in CI in separate PR. cc @FelixYBW

@PHILO-HE PHILO-HE merged commit 1c71001 into apache:main Feb 4, 2026
59 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Core] Memory leak in JNI_OnUnload due to missing protobuf shutdown

2 participants